Skip to content

Conversation

@maxkozlovsky
Copy link
Contributor

Remove using next pointers in rodb cache, always use hash, since using a shared ptr can keep nodes that should be recycled.

@maxkozlovsky maxkozlovsky force-pushed the max/singlenode branch 2 times, most recently from 917ec1c to cfdef25 Compare December 5, 2025 18:50
@maxkozlovsky maxkozlovsky marked this pull request as ready for review December 5, 2025 19:04
Copilot AI review requested due to automatic review settings December 5, 2025 19:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR consolidates the CacheNode and Node classes back into a single Node class, eliminating the need for separate node types in the read-only database cache. The change removes the complexity of managing two node types and simplifies the codebase by always using hash-based lookups instead of maintaining next pointers that could keep nodes alive longer than intended.

Key changes:

  • Merged NodeBase and removed CacheNode, consolidating all functionality into the Node class
  • Unified CacheNodeCursor and NodeCursor to use a single NodeCursor type with std::shared_ptr<Node>
  • Removed template parameters from deserialization functions (deserialize_node_from_receiver_result and deserialize_node_from_buffer)

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file
File Description
category/mpt/node.hpp Merged NodeBase into Node, removed CacheNode class and related template concepts, simplified copy_node and deserialize_node_from_buffer functions
category/mpt/node.cpp Updated all method implementations to use Node instead of NodeBase, removed CacheNode implementation
category/mpt/node_cursor.hpp Removed template-based NodeCursorBase and CacheNodeCursor, consolidated to single NodeCursor struct
category/mpt/node_cache.hpp Changed cache to store std::shared_ptr<Node> instead of std::shared_ptr<CacheNode>
category/mpt/deserialize_node_from_receiver_result.hpp Removed template parameter for node type, now always returns Node::SharedPtr
category/mpt/trie.hpp Updated type aliases for cursors and inflight maps to use NodeCursor instead of CacheNodeCursor
category/mpt/trie.cpp Removed template parameter from deserialization calls
category/mpt/traverse.hpp Removed template parameter from deserialization calls
category/mpt/read_node_blocking.cpp Removed template parameter from deserialization calls
category/mpt/find_request_sender.hpp Updated to use NodeCursor and Node throughout, removed next pointer cache logic in favor of hash-based lookups
category/mpt/find_notify_fiber.cpp Updated all cursor types and deserialization calls to use Node
category/mpt/db.hpp Changed API to use NodeCursor instead of CacheNodeCursor
category/mpt/db.cpp Updated implementation to use NodeCursor and Node throughout
category/mpt/test/node_lru_cache_test.cpp Simplified test to directly create Node instances without intermediate CacheNode conversion
category/mpt/test/monad_trie_test.cpp Updated benchmark code to use NodeCursor instead of CacheNodeCursor
category/mpt/test/mixed_async_sync_loads_test.cpp Simplified node cursor creation, removed copy_node call for CacheNode
category/mpt/test/db_test.cpp Updated async get operations and traverse to use Node instead of CacheNode
category/execution/ethereum/db/trie_rodb.hpp Changed prefix cursor type from CacheNodeCursor to NodeCursor

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Remove using next pointers in rodb cache, always use cache, since using
a shared ptr can keep nodes that should be recycled.

Remove copy_node definition, no longer necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants